chore: fix cryptography warning #207

sbor23 · 2021-02-09T10:20:25Z

Fix a warning emitted by cryptography since a recent release.

/usr/local/lib/python3.7/site-packages/jose/backends/cryptography_backend.py:18: CryptographyDeprecationWarning: int_from_bytes is deprecated, use int.from_bytes instead
  from cryptography.utils import int_from_bytes, int_to_bytes

Edit:
The warning comes from cryptography 3.4 release. Since this release only python 3.6+ is supported.
This PR fixes the warning by using int.from_bytes, which is only available since python 3.2.
So obviously python 2.7 is not supported anymore cryptography.

So the solutions for python-jose are:

drop python 2 support completely. This is the right solution IMO, it's 2021 after all.
pin the cryptography dep in setup.py to <3.4 if python 2 is detected and make the imports conditional. Defining the deps a bit more strictly would make sense anyways.

serhiy-storchaka · 2021-02-10T02:32:26Z

jose/backends/cryptography_backend.py

 from cryptography.hazmat.primitives.padding import PKCS7
 from cryptography.hazmat.primitives.serialization import load_pem_private_key, load_pem_public_key
-from cryptography.utils import int_from_bytes, int_to_bytes
+from cryptography.utils import int_to_bytes


If you can use int.from_bytes (it did not exist in 2.7), you can also use the to_bytes method of int.

~~True, but interestingly enough cryptography didn't bother to deprecate that one.~~
int_to_bytes is a custom implementation around int.to_bytes, so it's still needed.

Obviously this lifts the required python to 3.2+, but crypto itself has 3.6+ so this is necessary anyways.

I am facing the same issue.

serhiy-storchaka · 2021-02-10T09:35:42Z

I see tests are failed on 2.7. So conditional import is still needed.

sbor23 · 2021-02-10T09:50:51Z

I see tests are failed on 2.7. So conditional import is still needed.

As I said above, cryptography doesn't support python 2 anymore and instead requires 3.6+.
See the Changelog

sbor23 · 2021-02-10T14:13:48Z

Ok I have no idea what's going on with the CI.
Why does the py35-crypography-only stage pip uninstall rsa but still require rsa to be an importable module?
There seems to be some other issues doing on here which I am not willing to research or fix.
Tell me if there's an easy way to have this passing.

If it was about me, I would fix this simply by dropping support for python < 3.{3,6} for the whole project.

antdking · 2021-02-15T18:46:49Z

setup.py

+    elif platform.python_implementation() == 'CPython' and platform.python_version() < '3.6':
+        return 'cryptography < 3.4'


I don't think this is necessary.

Pip will resolve the correct version already based on cryptography's python_requires

$ python --version Python 3.5.9 $ pip --version pip 9.0.1 $ pip install python-jose[cryptography] ... Installing collected packages: pyasn1, rsa, six, ecdsa, pycparser, cffi, cryptography, python-jose Successfully installed cffi-1.14.5 cryptography-3.2.1 ecdsa-0.14.1 pyasn1-0.4.8 pycparser-2.20 python-jose-3.2.0 rsa-4.7 six-1.15.0

Note that on Python 3.5, even with an old version of pip, resolves cryptography to be 3.2.1 instead of 3.3.x or 3.4.x

Ah I see, good to know. I will drop this part then.

antdking · 2021-02-15T18:51:57Z

Why does the py35-crypography-only stage pip uninstall rsa but still require rsa to be an importable module?

It looks like there's a typo in your import, where you import cryptograph instead of cryptography, which will raise an ImportError.

RSA is the fallback if both cryptography and pycrypto fail with ImportErrors.

Fixing the typo should clear up the RSA error.

antdking · 2021-02-15T18:54:45Z

jose/backends/cryptography_backend.py

+    from cryptograph.utils import int_from_bytes, int_to_bytes
+else:
+    from cryptograph.utils import int_to_bytes


Suggested change

from cryptograph.utils import int_from_bytes, int_to_bytes

else:

from cryptograph.utils import int_to_bytes

from cryptography.utils import int_from_bytes, int_to_bytes

else:

from cryptography.utils import int_to_bytes

Thanks, done :)

cryptography 3.4 was breaking change, so pin to minor version.

codecov · 2021-03-01T10:38:06Z

Codecov Report

Merging #207 (3fe3c82) into master (8572088) will decrease coverage by 3.62%.
The diff coverage is 75.00%.

@@            Coverage Diff             @@
##           master     #207      +/-   ##
==========================================
- Coverage   93.51%   89.89%   -3.63%     
==========================================
  Files          16       16              
  Lines        1728     1731       +3     
==========================================
- Hits         1616     1556      -60     
- Misses        112      175      +63

Impacted Files	Coverage Δ
jose/backends/cryptography_backend.py	`91.10% <75.00%> (-2.11%)`	⬇️
jose/backends/_asn1.py	`63.15% <0.00%> (-26.32%)`	⬇️
jose/backends/rsa_backend.py	`86.33% <0.00%> (-9.32%)`	⬇️
jose/backends/pycrypto_backend.py	`87.10% <0.00%> (-5.93%)`	⬇️
jose/jwt.py	`96.83% <0.00%> (-3.17%)`	⬇️
jose/jws.py	`93.70% <0.00%> (-3.15%)`	⬇️
jose/jwe.py	`80.56% <0.00%> (-1.90%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8572088...3fe3c82. Read the comment docs.

sbor23 · 2021-03-15T13:03:59Z

Alright so I don't see which parts are dropping in coverage because the codecov repo requires authentication.
But it's obvious that the requirements are out of place an need fixing anyways.
python 2.7 doesn't work because rsa requires 3.6+, see CI example.

I strongly suggest to reconsider the decision to support anything < 3.6 and instead adopt #216 .

If you decide to support anything lower than that, feel free to fix the dependency pinning as I'm not going to do that.

ansipunk · 2021-03-18T11:59:17Z

@sbor23
Sign in to Codecov using your GitHub account to see the coverage report in details

aqeelat · 2021-03-21T20:38:00Z

Any updates on this PR?

nanotower · 2021-03-22T15:18:10Z

Any news? Also facing this issue in my app.

aqeelat · 2021-03-30T09:54:55Z

@sbor23 look at failing tests in:

The other failing builds should be removed from Travis if the maintainers decided to drop support for Python 2.7

sbor23 · 2021-03-30T09:57:50Z

@sbor23 look at failing tests in:
* https://travis-ci.org/github/mpdavis/python-jose/jobs/760919730

* https://travis-ci.org/github/mpdavis/python-jose/jobs/760919725
The other failing builds should be removed from Travis if the maintainers decided to drop support for Python 2.7

I know all of that and we're just waiting for maintainers to show up at that point. The CI is failing because of broken dependencies etc., as I mentiioned above.

asherf · 2021-05-01T18:33:31Z

fixed in #229

Release coming very soon.

cornel-masson · 2021-06-14T07:15:52Z

When is this scheduled for released? It's quite a nuisance in our logs.

sbor23 · 2021-06-14T07:48:21Z

When is this scheduled for released? It's quite a nuisance in our logs.

It was released under 3.3.0. See #260

amejias-manager mentioned this pull request Feb 9, 2021

Deprecation warning with cryptography 3.4 #208

Closed

serhiy-storchaka reviewed Feb 10, 2021

View reviewed changes

sbor23 force-pushed the fix-cryptography-warning branch from d80c2d1 to 69bad89 Compare February 10, 2021 09:28

sbor23 force-pushed the fix-cryptography-warning branch from 69bad89 to 65446d9 Compare February 10, 2021 12:42

antdking reviewed Feb 15, 2021

View reviewed changes

elchiapp mentioned this pull request Feb 23, 2021

Fix for deprecated int_from_bytes #212

Closed

chore: fix cryptography 3.4 warning

73bdecb

sbor23 force-pushed the fix-cryptography-warning branch from 65446d9 to 0e3c4af Compare March 1, 2021 10:00

fix(deps): cryptography version pinning

3fe3c82

cryptography 3.4 was breaking change, so pin to minor version.

sbor23 force-pushed the fix-cryptography-warning branch from 0e3c4af to 3fe3c82 Compare March 1, 2021 10:05

chitopunk mentioned this pull request Apr 13, 2021

Fix deprecation warning from cryptography_backend.py #222

Closed

asherf closed this May 1, 2021

matthewarmand mentioned this pull request Jun 3, 2021

bump version #248

Open

		elif platform.python_implementation() == 'CPython' and platform.python_version() < '3.6':
		return 'cryptography < 3.4'

chore: fix cryptography warning #207

chore: fix cryptography warning #207

Uh oh!

Conversation

sbor23 commented Feb 9, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

serhiy-storchaka Feb 10, 2021

Choose a reason for hiding this comment

Uh oh!

sbor23 Feb 10, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

06parv Feb 18, 2021

Choose a reason for hiding this comment

Uh oh!

serhiy-storchaka commented Feb 10, 2021

Uh oh!

sbor23 commented Feb 10, 2021

Uh oh!

sbor23 commented Feb 10, 2021

Uh oh!

antdking Feb 15, 2021

Choose a reason for hiding this comment

Uh oh!

sbor23 Mar 1, 2021

Choose a reason for hiding this comment

Uh oh!

antdking commented Feb 15, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

antdking Feb 15, 2021

Choose a reason for hiding this comment

Uh oh!

sbor23 Mar 1, 2021

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Mar 1, 2021

Codecov Report

Uh oh!

sbor23 commented Mar 15, 2021

Uh oh!

ansipunk commented Mar 18, 2021

Uh oh!

aqeelat commented Mar 21, 2021

Uh oh!

nanotower commented Mar 22, 2021

Uh oh!

aqeelat commented Mar 30, 2021

Uh oh!

sbor23 commented Mar 30, 2021

Uh oh!

asherf commented May 1, 2021

Uh oh!

cornel-masson commented Jun 14, 2021

Uh oh!

sbor23 commented Jun 14, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

sbor23 commented Feb 9, 2021 •

edited

Loading

sbor23 Feb 10, 2021 •

edited

Loading

antdking commented Feb 15, 2021 •

edited

Loading